-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
zfs-2.2.7 patchset #16720
base: zfs-2.2-release
Are you sure you want to change the base?
zfs-2.2.7 patchset #16720
Conversation
- Use the macros in few places it was missed. - Reduce scope of DB_DNODE_ENTER/EXIT() and inline some DB_DNODE() uses to make it more obvious what exactly is protected there and make unprotected accesses by mistake more difficult. - Make use of zrl_owner(). Reviewed-by: Rob Wing <[email protected] Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Allan Jude <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#16374
ZIL log record structs (lr_XX_t) are frequently allocated with extra space after the struct to carry variable-sized "payload" items. Linux 6.10+ compiled with CONFIG_FORTIFY_SOURCE has been doing runtime bounds checking on memcpy() calls. Because these types had no indicator that they might use more space than their simple definition, __fortify_memcpy_chk will frequently complain about overruns eg: memcpy: detected field-spanning write (size 7) of single field "lr + 1" at zfs_log.c:425 (size 0) memcpy: detected field-spanning write (size 9) of single field "(char *)(lr + 1)" at zfs_log.c:593 (size 0) memcpy: detected field-spanning write (size 4) of single field "(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0) memcpy: detected field-spanning write (size 7) of single field "lr + 1" at zfs_log.c:425 (size 0) memcpy: detected field-spanning write (size 9) of single field "(char *)(lr + 1)" at zfs_log.c:593 (size 0) memcpy: detected field-spanning write (size 4) of single field "(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0) memcpy: detected field-spanning write (size 7) of single field "lr + 1" at zfs_log.c:425 (size 0) memcpy: detected field-spanning write (size 9) of single field "(char *)(lr + 1)" at zfs_log.c:593 (size 0) memcpy: detected field-spanning write (size 4) of single field "(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0) To fix this, this commit adds flex array fields to all lr_XX_t structs that require them, and then uses those fields to access that end-of-struct area rather than more complicated casts and pointer addition. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Rob Norris <[email protected]> Closes openzfs#16501 Closes openzfs#16539
This sets RHEL8's base kernel[1] as the floor, and includes the oldest kernel.org LTS (4.19). 1. https://access.redhat.com/articles/3078#RHEL8 Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Rob Norris <[email protected]> Closes openzfs#16479
Update the META file to reflect compatibility with the 6.11 kernel. Reviewed-by: Rob Norris <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16586
Last commit should fix the underlying problem, so these should be passing reliably again. Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Rob Norris <[email protected]> Closes openzfs#16364
Optionally turn off disk's enclosure slot if an I/O is hung triggering the deadman. It's possible for outstanding I/O to a misbehaving SCSI disk to neither promptly complete or return an error. This can occur due to retry and recovery actions taken by the SCSI layer, driver, or disk. When it occurs the pool will be unresponsive even though there may be sufficient redundancy configured to proceeded without this single disk. When a hung I/O is detected by the kmods it will be posted as a deadman event. By default an I/O is considered to be hung after 5 minutes. This value can be changed with the zfs_deadman_ziotime_ms module parameter. If ZED_POWER_OFF_ENCLOSURE_SLOT_ON_DEADMAN is set the disk's enclosure slot will be powered off causing the outstanding I/O to fail. The ZED will then handle this like a normal disk failure. By default ZED_POWER_OFF_ENCLOSURE_SLOT_ON_DEADMAN is not set. As part of this change `zfs_deadman_events_per_second` is added to control the ratelimitting of deadman events independantly of delay events. In practice, a single deadman event is sufficient and more aren't particularly useful. Alphabetize the zfs_deadman_* entries in zfs.4. Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16226
Some libc's like uClibc lag the proper definition of SEEK_DATA and SEEK_HOLE. Since we have only two files in ZTS which use these definitons, let's define them by hand: ``` #ifndef SEEK_DATA #define SEEK_DATA 3 #endif #ifndef SEEK_HOLE #define SEEK_HOLE 4 #endif ``` There should be no failures, because: - FreeBSD has support for SEEK_DATA/SEEK_HOLE since FreeBSD 8 - Linux has it since Linux 3.1 - the libc will submit the parameters unchanged to the kernel Signed-off-by: Tino Reichardt <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: Allan Jude <[email protected]> Signed-off-by: Mateusz Piotrowski <[email protected]> Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Closes openzfs#16248
Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: Allan Jude <[email protected]> Signed-off-by: Mateusz Piotrowski <[email protected]> Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Closes openzfs#16248
Reviewed-by: George Melikov <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ameer Hamza <[email protected]> Closes openzfs#16432
The report generator expects the log to be clean and tidy UTF-8. That can be a problem if you use some of the verbose/debug test runner options, which sends all sorts of weird output from arbitrary programs to the log. This just makes Python a little more relaxed about such things. It shouldn't matter in practice, as those lines didn't match the test result regex anyway, and are discarded immediately. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Tony Hutter <[email protected]>
The test needs some adjusting within the timings. Reviewed by: Brian Behlendorf <[email protected]> Signed-off-by: Tony Hutter <[email protected]> Co-authored-by: Tino Reichardt <[email protected]> Closes openzfs#16537
On load the test needs sometimes a bit more time then just one second. Doubling the time will help on the QEMU based testings. Reviewed by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16537
This commit adds functional tests for these systems: - AlmaLinux 8, AlmaLinux 9, ArchLinux - CentOS Stream 9, Fedora 39, Fedora 40 - Debian 11, Debian 12 - FreeBSD 13, FreeBSD 14, FreeBSD 15 - Ubuntu 20.04, Ubuntu 22.04, Ubuntu 24.04 - enabled by default: - AlmaLinux 8, AlmaLinux 9 - Debian 11, Debian 12 - Fedora 39, Fedora 40 - FreeBSD 13, FreeBSD 14 Workflow for each operating system: - install qemu on the github runner - download current cloud image of operating system - start and init that image via cloud-init - install dependencies and poweroff system - start system and build openzfs and then poweroff again - clone build system and start 2 instances of it - run functional testings and complete in around 3h - when tests are done, do some logfile preparing - show detailed results for each system - in the end, generate the job summary Real-world benefits from this PR: 1. The github runner scripts are in the zfs repo itself. That means you can just open a PR against zfs, like "Add Fedora 41 tester", and see the results directly in the PR. ZFS admins no longer need manually to login to the buildbot server to update the buildbot config with new version of Fedora/Almalinux. 2. Github runners allow you to run the entire test suite against your private branch before submitting a formal PR to openzfs. Just open a PR against your private zfs repo, and the exact same Fedora/Alma/FreeBSD runners will fire up and run ZTS. This can be useful if you want to iterate on a ZTS change before submitting a formal PR. 3. buildbot is incredibly cumbersome. Our buildbot config files alone are ~1500 lines (not including any build/setup scripts)! It's a huge pain to setup. 4. We're running the super ancient buildbot 0.8.12. It's so ancient it requires python2. We actually have to build python2 from source for almalinux9 just to get it to run. Ugrading to a more modern buildbot is a huge undertaking, and the UI on the newer versions is worse. 5. Buildbot uses EC2 instances. EC2 is a pain because: * It costs money * They throttle IOPS and CPU usage, leading to mysterious, * hard-to-diagnose, failures and timeouts in ZTS. * EC2 is high maintenance. We have to setup security groups, SSH * keys, networking, users, etc, in AWS and it's a pain. We also * have to periodically go in an kill zombie EC2 instances that * buildbot is unable to kill off. 6. Buildbot doesn't always handle failures well. One of the things we saw in the past was the FreeBSD builders would often die, and each builder death would take up a "slot" in buildbot. So we would periodically have to restart buildbot via a cron job to get the slots back. 7. This PR divides up the ZTS test list into two parts, launches two VMs, and on each VM runs half the test suite. The test results are then merged and shown in the sumary page. So we're basically parallelizing ZTS on the same github runner. This leads to lower overall ZTS runtimes (2.5-3 hours vs 4+ hours on buildbot), and one unified set of results per runner, which is nice. 8. Since the tests are running on a VM, we have much more control over what happens. We can capture the serial console output even if the test completely brings down the VM. In the future, we could also restart the test on the VM where it left off, so that if a single test panics the VM, we can just restart it and run the remaining ZTS tests (this functionaly is not yet implemented though, just an idea). 9. Using the runners, users can manually kill or restart a test run via the github IU. That really isn't possible with buildbot unless you're an admin. 10. Anecdotally, the tests seem to be more stable and constant under the QEMU runners. Reviewed by: Brian Behlendorf <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Signed-off-by: Tony Hutter <[email protected]> Closes openzfs#16537
On larger files this should improve the speed. Sample values of my system: [mcmilk@xz]$ time dd if=/dev/zero bs=128k count=1k | sha256sum 254bcc3fc4f27172636df4bf32de9f107f620d559b20d760197e452b97453917 - real 0m1,050s user 0m0,985s sys 0m0,153s [mcmilk@xz]$ time dd if=/dev/zero bs=128k count=1k | openssl sha256 -r 254bcc3fc4f27172636df4bf32de9f107f620d559b20d760197e452b97453917 *stdin real 0m0,254s user 0m0,206s sys 0m0,160s I think cli_root/zdb/zdb_backup.ksh runs also an FreeBSD and I needed to include the sysutils/coreutils package for the FreeBSD tests within the QEMU patchset. This could be reverted, when this pull request gets upstream Reviewed-by: George Melikov <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16543
Fix that error: "cat /tmp/failed.txt: No such file or directory" Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16549
This commit changes the workflow of the github actions. - Ubuntu 20.04, 22.04, 24.04 will be tested via QEMU now - remove unused scripts of this commit: b7bc334 - re-add the zloop standalone testings via zloop.yml Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16549
In zpool_create.shlib, check_feature_set iterates over all features mentioned in provided compatibility file to check if only those features are enabled on the pool. This commit fixes skipping over comment lines correctly. Otherwise, the test case fails as comment lines are also treated as feature names by check_feature_set function. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Umer Saleem <[email protected]> Closes openzfs#15909
The qemu-9-summary-page.sh script reads the file env.txt in the first lines. When the module didn't build, this file was not copied into the tarfile - causing the scipt to abort. Fix: copy needed files into the tarfile in case of module build failures. The fix ignores also empty tarfiles in future. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16555
There is no longer be a need for the ci_reason exception with the update CI GitHub Actions infrastruture. Retire it. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16553
All supported Linux kernels, 4.18 and newer, provide O_TMPFILE. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16553
The following tests have been observed to occasionally fail when running under the CI. Updated our exceptions list to track them. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16553
Switch from v2 to v3 CodeQL Actions. The v2 actions will no longer be supported as of Dec '24 so we need to move to v3. According to the release notes they should be functionally equivalent. Note that the only difference between v2 and v3 of the CodeQL Action is the node version they support, ... For example 3.22.11 was the first v3 release and is functionally identical to 2.22.11. https://github.com/github/codeql-action/blob/main/CHANGELOG.md Reviewed-by: George Melikov <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16560
Update the CONTRIBUTING.md documentation to refer to the GitHub Actions workflows which have replaced the buildbot infrastructure. Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16561
For checkstyle, zloop, zfs-qemu, and codeql workflows cancel in-progress jobs when the PR is updated. Relevant GitHub Actions documentation: The following concurrency group cancels in-progress jobs or run on pull_request events only; if github.head_ref is undefined, the concurrency group will fallback to the run ID, which is guaranteed to be both unique and defined for the run. https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#example-using-a-fallback-value Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16562
The commit uses heuristics to determine whether a PR is behavioral: It runs "quick" CI (i.e., only use sanity.run on fewer OSes) if (explicitly requested by user): - the *last* commit message contains a line 'ZFS-CI-Type: quick', or if (by heuristics): - the files changed are not in the list of specified directory, and - all commit messages does not contain 'ZFS-CI-Type: full'. It runs "full" CI otherwise. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Shengqi Chen <[email protected]> Closes openzfs#16564
Update the test case to freeze the pool then export it to better simulate a hard failure. This is preferable to copying the vdev while the pool's imported since with a copy we're not guaranteed the on-disk state will be consistent. That can in turn result in a pool import failure and a spurious test failure. Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16570
On failure attempt to include the most relevant portions of the ztest logs in the CI output. This full logs are still available for download but often a backtrace and the last output is enough. Install libunwind to improve the odds of a useful backtrace. Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16573
Lower the minimum number of expected deadman events from 4 to 3. All that is strictly required is a single event to consider the test a pass. However, since I've never seen a count of less than 3 reported by the CI that should be sufficient. Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: George Melikov <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16575
Update the test case to freeze the pool then export it to better simulate a hard failure. This is preferable to copying the vdev while the pool's imported since with a copy we're not guaranteed the on-disk state will be consistent. That can in turn result in a pool import failure and a spurious test failure. Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16578
linux/torvalds@11068e0b64cb removes it, suggesting this was a always there as a helper to handle concurrent seeks, which all filesystems now handle themselves if necessary. Without looking into the mechanism, I can imagine how it might have been used, but we have always set it to zero and never read from it, presumably because we've always tracked per-caller position through the znode anyway. So I don't see how there can be any functional change for us by removing it. I've stayed conservative though and left it in for older kernels, since its clearly not hurting anything there. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Rob Norris <[email protected]> Closes openzfs#16582
torvalds/linux@09022bc196d2 removes the flag, and the corresponding SetPageError() and ClearPageError() macros, with no replacement offered. Going back through the upstream history, use of this flag has been gradually removed over the last year as part of the long tail of converting everything to folios. Interesting tidbit comments from torvalds/linux@29e9412b250e and torvalds/linux@420e05d0de18 suggest that this flag has not been used meaningfully since page writeback failures started being recorded in errseq_t instead (the whole "fsyncgate" thing, ~2017, around torvalds/linux@8ed1e46aaf1b). Given that, it's possible that since perhaps Linux 4.13 we haven't been getting anything by setting the flag. I don't know if that's true and/or if there's something we should be doing instead, but my gut feel is that its probably fine we only use the page cache as a proxy to allow mmap() to work, rather than backing IO with it. As such, I'm expecting that removing this will do no harm, but I'm leaving it in for older kernels to maintain status quo, and if there is an overall better way, that is left for a future change. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Rob Norris <[email protected]> Closes openzfs#16582
bb2523d
to
213b5e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues here
I have this PR running since 3 days on kernel 6.6.x on Endeavouros without any issue. |
Looks like module load isn't working on FreeBSD:
I'm looking into it |
Being able to print custom debug information on assert trip seems useful. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Paul Dagnelie <[email protected]> Signed-off-by: Rich Ercolani <[email protected]> Closes openzfs#15792
The FreeBSD linux/compiler.h in OpenZFS was copied from a very old version of FreeBSD's linuxkpi's linux/compiler.h. There's no need for this duplication. Use FreeBSD's linuxkpi version instead, and provide zfs_fallthrough to augment it (it's all that's needed). Use #pragma once to avoid naming issues for guard variables. Since this is a complete rewrite, use my copyright here (the original code in FreeBSD still credits everybody). This works back at least to FreeBSD 12.4, which is not out of support, and all newer releases. Remove extra copies of macros that were defined elsewhere, but are now properly defined in LinuxKPI so are redundant. Sponsored-by: Netflix Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Warner Losh <[email protected]> Closes openzfs#16650
With FreeBSD's switch to git the $FreeBSD$ string is no longer expanded and they have mostly been removed upstream. Stop using __FBSDID and remove the no-longer needed sys/cdefs.h includes. Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Brooks Davis <[email protected]> Closes openzfs#15527
These members have directly references to the global variables exposed by the kernel. They are not going to be changed by this kernel module. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Zhenlei Huang <[email protected]> Closes openzfs#16210
213b5e8
to
379d46d
Compare
@tonyhutter You want 80645d6. |
I accidentally removed this in c22d56e, and didn't notice because it doesn't fail the build, but does fail to load into the kernel because it can't link it. Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Rob Norris <[email protected]> Closes openzfs#16554
379d46d
to
0973ff7
Compare
@robn thanks, I included it in my latest push. |
Do you want to add 6.12 compatibility with PR #16793 ? |
@tonyhutter I think it would be great to have 38c0324 included, as a lot of 2.2 users experience the underlying issue. |
Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: Umer Saleem <[email protected]> Reviewed-by: Ameer Hamza <[email protected]> Reviewed-by: Tony Hutter <[email protected]>
On 64bit FreeBSD this reduces one from 296 to 280 bytes. On small block workloads dbufs may consume gigabytes of ARC, and this saves 5% of it. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: Brian Atkinson <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#16684
If we write less than 113 bytes with enabled compression we get embeded block, which then fails check for number of cloned blocks in bclone_test. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Pawel Jakub Dawidek <[email protected]> Reviewed-by: Brian Atkinson <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#16740
We are doing exactly the same checks around all brt_pending_add(). Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Pawel Jakub Dawidek <[email protected]> Reviewed-by: Brian Atkinson <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#16740
Update the META file to reflect compatibility with the 6.12 kernel. Reviewed-by: Umer Saleem <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16793
by protecting against sb->s_shrink eviction on umount with newer kernels deactivate_locked_super calls shrinker_free and only then sops->kill_sb cb, resulting in UAF on umount when trying to reach for the shrinker functions in zpl_prune_sb of in-umount dataset Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Adam Moss <[email protected]> Signed-off-by: Pavel Snajdr <[email protected]> Closes openzfs#16770
META file and changelog updated. Signed-off-by: Tony Hutter <[email protected]>
0973ff7
to
b1f6139
Compare
For full 6.11+ kernel compatibility I guess we'd also need #16805 - once it's merged.... |
Motivation and Context
Description
Proposed zfs-2.2.7 patchset. Support 6.11 kernel and github QEMU runners.
How Has This Been Tested?
Runners will test
Types of changes
Checklist:
Signed-off-by
.